ASoC: SOF: ipc4-topology: Allow bytes controls without initial payload#5708
Conversation
ujfalusi
requested review from
bardliao,
dbaluta,
kv2019i,
lgirdwood,
plbossart and
ranj063
as code owners
There was a problem hiding this comment.
Pull request overview
This PR adjusts the IPC4 SOF topology bytes-control loader to accept bytes controls that have no initial private payload, matching the stated topology constraints and the driver’s intended handling of “no initial data” cases.
Changes:
- Relax the minimum-size validation so
priv_size == 0is permitted for bytes controls (previously rejected as “insufficient”).
Comments suppressed due to low confidence (1)
sound/soc/sof/ipc4-topology.c:2978
- Allowing priv_size == 0 here means we can allocate a bytes control with an all-zero sof_abi_hdr in control_data->data (kzalloc). Later code uses data->type to build msg->extension (e.g., control_load_bytes() itself sets msg->extension from control_data->data->type, and bytes_ext_volatile_get() / refresh paths call sof_ipc4_set_get_bytes_data() / sof_ipc4_refresh_bytes_control() which also rely on data->type). With type left at 0, GET operations can send an invalid param_id (0) and user-visible reads may return an uninitialized ABI header (magic/type/abi all zero). Consider initializing the ABI header fields when priv_size==0 (at least magic and a valid type/param_id if it can be derived), and/or gating GET/refresh paths to avoid issuing IPC until a valid type is known.
if (scontrol->priv_size && scontrol->priv_size < sizeof(struct sof_abi_hdr)) {
dev_err(sdev->dev,
"bytes control %s initial data size %zu is insufficient.\n",
scontrol->name, scontrol->priv_size);
return -EINVAL;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
kv2019i
left a comment
kv2019i
left a comment
There was a problem hiding this comment.
Typo in commit message (s/fro/for), otherwise looks good.
It is unexpected, but allowed to have no initial payload for a bytes control and the code is prepared to handle this case, but the size check missed this corner case. Update the check for minimal size to allow the initial size to be 0. Cc: stable@vger.kernel.org Fixes: a653820 ("ASoC: SOF: ipc4-topology: Correct the allocation size for bytes controls") Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
ujfalusi
force-pushed
the
peter/sof/pr/ipc4-topology-bytes-no-initial-data
branch
from
16e6b29 to
290e3e3
Compare
singalsu
left a comment
singalsu
left a comment
There was a problem hiding this comment.
I tried removed bytes control initialization, got a topology parse fail, while with this patch it didn't happen.
|
@ujfalusi , fri, verified, dax topology loaded successfully. |
|
It looks like If so, should we make dax, i.e. the failed case, escape from include/components/dolby-dax.conf: include/components/dax/preset.conf: or, directly add to include/controls/bytes.conf to prevent the future cases? include/controls/bytes.conf |
|
@johnylin76, it is unexpected that no initial data is provided for the bytes control, but the rest of the code handles this in kernel. |
I see. It's hard to tell which one is better. I think a module owner should follow the standard way to prevent any unexpected error. Despite that, I think this patch should be taken because for an affected control on the production device which has no initial data, it has no way to fix on runtime but update the topology. |
It is unexpected, but allowed to have no initial payload for a bytes control and the code is prepared to handle this case, but the size check missed this corner case.
Update the check fro minimal size to allow the initial size to be 0.
Cc: stable@vger.kernel.org
Fixes: a653820 ("ASoC: SOF: ipc4-topology: Correct the allocation size for bytes controls")